Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Poll for results from frontend for async search #8481

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

amsiglan
Copy link
Collaborator

@amsiglan amsiglan commented Oct 4, 2024

Description

Currently for async search we depend on the search strategy to start the query and poll for results and resolve once the query completes/fails but since can to the order of 1 to 2 min, the frontend will timeout. This PR adds logic in frontend to poll for the results once the query is started.

The search strategy's search method will be invoked with the query id when polling for results, else it should start search for the given query.

Testing the changes

Tested using local setup and neo app

Changelog

  • feat: Add logic to poll for async query result

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

AMoo-Miki
AMoo-Miki previously approved these changes Oct 4, 2024
Copy link
Member

@ashwin-pc ashwin-pc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, love the changes. Can you also update the SQL search strategy to use this. Right now this change breaks the SQL strategy when querying remote sources like this.

@ashwin-pc
Copy link
Member

Screenshot 2024-10-04 at 4 53 02 AM

For reference

Copy link

codecov bot commented Oct 4, 2024

Codecov Report

Attention: Patch coverage is 3.22581% with 30 lines in your changes missing coverage. Please review.

Project coverage is 60.91%. Comparing base (e077644) to head (2955406).

Files with missing lines Patch % Lines
.../data/common/search/search_source/search_source.ts 0.00% 16 Missing ⚠️
src/plugins/data/common/utils/helpers.ts 8.33% 11 Missing ⚠️
...ic/application/view_components/utils/use_search.ts 0.00% 2 Missing ⚠️
src/plugins/query_enhancements/common/utils.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8481      +/-   ##
==========================================
- Coverage   60.93%   60.91%   -0.02%     
==========================================
  Files        3760     3761       +1     
  Lines       89301    89325      +24     
  Branches    13969    13977       +8     
==========================================
+ Hits        54412    54413       +1     
- Misses      31493    31516      +23     
  Partials     3396     3396              
Flag Coverage Δ
Linux_1 28.89% <3.33%> (-0.01%) ⬇️
Linux_2 56.25% <3.57%> (-0.05%) ⬇️
Linux_3 37.74% <3.33%> (-0.02%) ⬇️
Linux_4 29.92% <3.44%> (-0.02%) ⬇️
Windows_1 28.91% <3.33%> (-0.01%) ⬇️
Windows_2 56.20% <3.57%> (-0.05%) ⬇️
Windows_3 37.75% <3.33%> (-0.02%) ⬇️
Windows_4 29.92% <3.44%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@amsiglan
Copy link
Collaborator Author

amsiglan commented Oct 4, 2024

Fixed the async sql strategy and also the issue that we see Cannot read message of undefined

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants